Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tailcut finder function and script #1325

Merged
merged 45 commits into from
Jan 27, 2025
Merged

Tailcut finder function and script #1325

merged 45 commits into from
Jan 27, 2025

Conversation

moralejo
Copy link
Collaborator

@moralejo moralejo commented Dec 16, 2024

Find adequate cleaning levels (and MC NSB tuning settings) to process a given run, just based on the pixel charges extracted from interleaved pedestal events. See #1330 for some tests of performance.

@moralejo
Copy link
Collaborator Author

@marialainez, @morcuended this is still a draft (I am testing it), but it would be good if you can already comment on the interface, since this is mainly to use by lstosa

Copy link

codecov bot commented Dec 16, 2024

Codecov Report

Attention: Patch coverage is 20.86331% with 110 lines in your changes missing coverage. Please review.

Project coverage is 72.94%. Comparing base (9aaa78b) to head (563bc26).
Report is 87 commits behind head on main.

Files with missing lines Patch % Lines
lstchain/image/cleaning.py 15.62% 81 Missing ⚠️
lstchain/scripts/lstchain_find_tailcuts.py 32.55% 29 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1325      +/-   ##
==========================================
- Coverage   73.52%   72.94%   -0.58%     
==========================================
  Files         134      135       +1     
  Lines       14215    14350     +135     
==========================================
+ Hits        10451    10468      +17     
- Misses       3764     3882     +118     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…itional NSB needed in the MC.

Previous ones were based on the mean pedestal charges, with no exclusion
of outliers (pixels or subruns). New ones are based on the medians computed
in the current code (& excluding outliers)
@moralejo moralejo marked this pull request as ready for review December 17, 2024 16:25
@moralejo moralejo marked this pull request as ready for review December 19, 2024 10:25
@moralejo moralejo marked this pull request as draft December 19, 2024 16:32
pixels, but truncating the charge distribution of all pixels (hence biasing
the result)
(to those obtained with the proper pixel outlier exclusion in MC)
…e, which

characterizes better the right-side tail of the distribution (more relevant for
the analysis)
@moralejo moralejo marked this pull request as ready for review December 21, 2024 08:57
@moralejo moralejo requested a review from vuillaut December 21, 2024 08:57
@moralejo
Copy link
Collaborator Author

This is now ready for review; performance results and some further explanations are shown in #1330

@moralejo
Copy link
Collaborator Author

Below is the correlation between the outputs of find_tailcuts, picture threshold vs. additional NSB rate, compared to the settings of the recently produced standard MC simulation NSB grid.
image

To avoid confusion, we set the "tailcut" setting in the json file (which is not used later) to {} (empty), rather than the default 8,4 (picture, boundary). The default appeared in the attrs of the resulting DL1 file, which may lead to confusion. The actually used settings are those in tailcuts_clean_with_pedestal_threshold
Copy link
Member

@morcuended morcuended left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall. Just some minor comments regarding docstrings. And also if the script is calculating the additional NSB, I'd indicate it in the name of the script.

lstchain/image/cleaning.py Outdated Show resolved Hide resolved
lstchain/image/cleaning.py Outdated Show resolved Hide resolved
lstchain/image/cleaning.py Outdated Show resolved Hide resolved
lstchain/image/cleaning.py Outdated Show resolved Hide resolved
lstchain/image/cleaning.py Outdated Show resolved Hide resolved
lstchain/scripts/lstchain_find_tailcuts.py Show resolved Hide resolved
lstchain/image/cleaning.py Outdated Show resolved Hide resolved
lstchain/image/cleaning.py Show resolved Hide resolved
lstchain/image/cleaning.py Outdated Show resolved Hide resolved
lstchain/image/cleaning.py Outdated Show resolved Hide resolved
Copy link
Member

@morcuended morcuended left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My last concern is how to deal with edge cases like very short runs for which this script will fail. But that's more of an issue for OSA.

@moralejo
Copy link
Collaborator Author

My last concern is how to deal with edge cases like very short runs for which this script will fail. But that's more of an issue for OSA.

The parameters are calculated for a whole run (not a subrun), so it really has to be a very short run, < 1 minute or so. That usually means something went very wrong, and data are not usable.

@morcuended
Copy link
Member

These very short runs (<5 subruns) are created now and then as leftovers when observations on a given target are finishing (I guess due to data acquisition configuration). Their case should be considered by OSA, like simply excluding them from the analysis from the beginning. Not really a problem with this script.

@moralejo moralejo merged commit 0e1ab4d into main Jan 27, 2025
6 of 8 checks passed
@moralejo moralejo deleted the tailcut_finder branch January 27, 2025 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants